Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Allow entity's methods calls #13

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

antonioribeiro
Copy link

No description provided.

@GrahamCampbell
Copy link
Contributor

Wait, what? Isn't there an oversite here? This will change behaviour for getting relationships won't it? Because say if an object belongsTo a user, then user() is different to user?

Surely, a better solution would be to add a __call to call the methods on the model?

@antonioribeiro
Copy link
Author

Sorry. I will have to look better into this.

@antonioribeiro antonioribeiro deleted the patch-1 branch July 23, 2014 18:15
@antonioribeiro antonioribeiro restored the patch-1 branch July 23, 2014 18:16
@GrahamCampbell
Copy link
Contributor

I still don't get why this has to go in the __get function???

@antonioribeiro
Copy link
Author

So people can access methods using:

{{ $entity->present()->methodName }}

and not

{{ $entity->present()->methodName() }}

And not really think about it being a method or a property in the model.

Of course people can just create a acessors (getPropertyNameAttribute), but this way they will also be able to create simple methods and, (only) from the presenter, access them as a properties.

This is exactly what Eloquent does for relationships, isn't it? This should be exclusively used for relationships?

My use case was for a rather complex method, which should be done with a relationship, but couldn't due to some limitations, so I grab some data and do some stuff in that method and return the collection I need. Not really the case for an accessor, imo.

Of course I can extend the Laracasts\Presenter and just use my own, but I'm trying to contribute, because some other people may encounter the same problem.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants